Spatially-distributed channel subsets for the sweeps (#91) - #99
Merged
Conversation
Member
Author
Review (Sonnet agents, 2 parallel reviewers)Reviewed Important (fixed)
Coverage (added)
Suggestion (intentional, not changed)
Verified correct (no change)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #91.
The dimension sweeps reduced channel count by slicing
full[:nc]-- the first nc electrodes in file order, which for ds002718 (EEG001..) is a spatial cluster (one scalp region), not a whole-head cap. So reduced-channel decompositions and their IC scalp maps were not physically meaningful reduced montages (visible in the Phase 3 16ch topomaps, #87).What changed
benchmarks/channel_selection.py(new): parse a BIDSelectrodes.tsvand pick nc spatially-distributed channels via greedy farthest-point (k-center) sampling over the real 3D electrode positions. Deterministic (seeded from the centroid-nearest electrode). Unlocalized (n/a) channels are excluded.benchmark_decompose.py: when--montageis given, reduced channel counts use the distributed subset for the decomposition; the selected channel indices are saved in each npz and threaded through_compare/_plot_topomaps/_load_infoso the topomaps and the de-sphere use the correct electrodes (notEEG001..EEG_nc).benchmark_dimsweep.py: same--montagedistributed selection at the slice (channel count unchanged, so timing is unaffected; only the montage becomes physical).pyproject.toml: newvizextra (mne) for the topomap/montage path, withuv.lockupdated.Tests
pyAMICA/tests/test_channel_selection.py(6 tests) against a real committed electrode fixture (ds002718 sub-002, CC0): parsing, deterministic/unique/sorted selection, and the key property that a distributed subset is more spread (larger nearest-pair distance) than the first-N cluster. Also surfaced a real data property: EEG061-064 are external channels with no scalp position, so distributed subsets automatically exclude the non-scalp externals (66 localized of 70). Full suite green.Remaining (validation, not code)
Re-run the channel sweep on hallu with
--montageto regenerate the Phase 3 topomaps as proper whole-head reduced montages (timing/equivalence numbers unchanged; only the maps become physically sensible). Tracked as the follow-up in this PR.Tested
14 relevant tests pass (
test_channel_selection,test_decompose_equiv,test_dimsweep_threads); ruff clean.